fix: detect WERR_REVIEW_ACTIONS by duck-type, not constructor.name - #44
Merged
sirdeggen merged 2 commits intoMay 7, 2026
Conversation
The Vite production build (esbuild minification) renames class names,
so `error?.constructor.name === 'WERR_REVIEW_ACTIONS'` returns mangled
identifiers and the check fails at runtime. Errors then fall through
to the generic `{message: ...}` wrapper which strips `code: 5`,
`reviewActionResults`, `sendWithResults`, `txid`, `tx`, and
`noSendChange` from the response body.
The calling app's `@bsv/sdk` HTTPWalletJSON dispatch checks
`data.code === 5` to reconstruct WERR_REVIEW_ACTIONS with the signed
transaction and review reasons. Without `code: 5` it falls through to
its own generic Error wrapper, leaving the app unable to recover the
signed tx or surface review reasons to the user — so flows that use
`acceptDelayedBroadcast: false` and hit any review-actions condition
(serviceError, doubleSpend, invalidTx) become un-debuggable.
Replace the constructor.name check with a duck-type predicate that
matches on the canonical message + presence of the
`reviewActionResults` array. This is robust to bundler minification
and to the cross-package class identity issue (errors thrown by
`@bsv/wallet-toolbox`'s WERR_REVIEW_ACTIONS class are not
`instanceof @bsv/sdk`'s WERR_REVIEW_ACTIONS class — they are
separate class declarations).
Applies to all three handlers: createAction, signAction,
internalizeAction.
Tests: full build clean (renderer + electron), all existing tests
pass (translations + onWalletReady — 25 total).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Thanks for the contribution. Taking a look |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes renderer-side detection of WERR_REVIEW_ACTIONS errors in src/onWalletReady.ts so that Vite/esbuild minification doesn’t break error classification and strip important structured fields (code, reviewActionResults, tx, etc.) from HTTP responses returned to calling apps.
Changes:
- Replaced
error?.constructor.name === 'WERR_REVIEW_ACTIONS'checks with a newisWerrReviewActions(...)duck-typing predicate. - Updated the
createAction,signAction, andinternalizeActionerror branches to use the new predicate before re-wrapping into@bsv/sdk’sWERR_REVIEW_ACTIONS.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The renderer-side handler in
src/onWalletReady.tsuseserror?.constructor.name === 'WERR_REVIEW_ACTIONS'to detect review-actions errors at three sites (createAction, signAction, internalizeAction). The Vite production build (esbuild minification) renames class identifiers, so at runtimeconstructor.namereturns mangled values like'a'and the check returns false. Errors then fall through to the genericelsebranch which only emits{message: ...}— droppingcode: 5,reviewActionResults,sendWithResults,txid,tx, andnoSendChangefrom the response body.The calling app's
@bsv/sdkHTTPWalletJSONchecksdata.code === 5(and matching siblings 6, 7, 8) to reconstruct the properWERR_REVIEW_ACTIONSinstance with the signed transaction and review reasons. Withoutcode: 5reaching the wire, the SDK falls through to its own generic Error wrapper, leaving the calling app unable to:txAtomicBEEF) for fallback broadcast through other providers'serviceError' | 'doubleSpend' | 'invalidTx') to the userThis makes any flow using
acceptDelayedBroadcast: falseun-debuggable when the bundled broadcaster encounters a review condition.How discovered
Hit while integrating BRC-100 wallet support in a DEX (BSV-21 mint flow with ~5KB embedded-icon deploy script). With BSV Desktop v2.2.7 the createAction returned 400 with body
{"message":"Undelayed createAction or signAction results require review."}— nocode, notx, noreviewActionResults. With MetaNet Desktop on the same flow + script size, the SDK reconstructedWERR_REVIEW_ACTIONScorrectly viacase 5:dispatch, surfacing the review reason and providing the signed tx for manual fallback broadcast.Verified the JSON.stringify produces the expected fields when isolated:
```js
const e = new sdk.WERR_REVIEW_ACTIONS([{txid:'a',status:'serviceError'}], [{txid:'a',status:'sending'}], 'a', [1,2,3], ['a.0']);
console.log(JSON.stringify(e));
// {"reviewActionResults":[...],"sendWithResults":[...],"txid":"a","tx":[1,2,3],"noSendChange":["a.0"],"isError":true,"code":5,"name":"WERR_REVIEW_ACTIONS"}
```
So the serialization itself is fine — the bug is the detection check failing under minification, which routes the error to the wrong branch entirely.
Fix
Replace the brittle
constructor.namestring check with a duck-type predicate matching on the canonical error message + presence of thereviewActionResultsarray:```ts
function isWerrReviewActions(error: unknown): error is { reviewActionResults: unknown[] } {
if (typeof error !== 'object' || error === null) return false;
const e = error as { message?: unknown; reviewActionResults?: unknown };
return (
e.message === 'Undelayed createAction or signAction results require review.' &&
Array.isArray(e.reviewActionResults)
);
}
```
This is robust to:
@bsv/wallet-toolbox'sWERR_REVIEW_ACTIONSclass but checked here against an import of@bsv/sdk'sWERR_REVIEW_ACTIONSclass — these are separate class declarations, soinstanceofwould not work as a replacement either.The duck-type check fires for both wallet-toolbox-thrown and SDK-thrown errors that genuinely match the WERR_REVIEW_ACTIONS shape, and would not false-positive on unrelated errors (the canonical message string is unique to this error type).
Test plan
🤖 Generated with Claude Code